-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New feature: local cache #35
Conversation
c91033f
to
2eee140
Compare
Update a little test: Complie source code and get request "http://httpbin.ogr/anything"
It speed up 3x fast! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good feature, this also needs specs.
I find Halite::Features
and Halite::Feature
are quite confusing.
I think having directly Halite::Logger
and Halite::Cache
would clearer.
src/halite/features/cache.cr
Outdated
# r.headers # => {..., "X-Cached-At" => "2018-08-30 10:41:14 UTC", "X-Cached-By" => "Halite", "X-Cached-Expires-At" => "2018-08-30 10:41:19 UTC", "X-Cached-Key" => "2bb155e6c8c47627da3d91834eb4249a"}} | ||
# ``` | ||
class Cache < Feature | ||
DEFAULT_PATH = "cache/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not a good idea to create a directory in the root directory, at least create in /tmp
Maybe a better approach would be:
- On a new
Cache
object creation, a temporary directory will be created with https://crystal-lang.org/api/master/Tempfile.html (Tempfile.new("halite-cache").path
) - Add a
finalize
method that will delete the directory when the Object is destructed (#delete
)
Note: the API will likely change in the next release crystal-lang/crystal#6485
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit just a simple test, it was not following my spec. May be push it early.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tempfile
can be used for the cache
That's my problem, i did not design the picture enough, I'm working on it, i will removing Features and let feature under Halite module. It will a new PR to do (#36) |
…ture under Halite and some method changes. Feature: - `Halite::Features::Logger` to `Halite::Logger` - `Halite::Features.register` to `Halite.register_feature` - `Halite::Features[]/[]?` to `Halite.features/features?` - `Halite::Features.avaiables` to `Halite.has_feature?` - `Halite::Feature::Interceptor::Chain` to `Halite::Feature::Chain`
…d merge Halite::MimeTypes.register_adapter/register_alias into Halite::MimeType.register
…d merge Halite::MimeTypes.register_adapter/register_alias into Halite::MimeType.register
…hmark to json format
1b614ba
to
423c4db
Compare
423c4db
to
8842c48
Compare
Halite.use("cache", file: "github_users.json").get("https://api.github.com/v3/users")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready to merge 🎉
Related to #24